Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #13920: Fixed erroneous validation for specific cases #19909

Conversation

tim-fischer-maschinensucher
Copy link
Contributor

Q A
Is bugfix? ✔️
New feature?
Breaks BC?
Fixed issues #13920

@what-the-diff
Copy link

what-the-diff bot commented Jul 24, 2023

PR Summary

  • Correction of validation process for specific scenarios
    The PR includes fixes to the validation process, enhancing it to handle a wider range of cases and prevent any mistakes more effectively.

  • Optimization of the GridView label generation
    Previously, there were instances where GridView unnecessarily called the Model::generateAttributeLabel() function to produce label values that aren't required. This PR fixes this to improve overall efficiency.

  • Issue with caching a MSSQL query containing BLOB data type resolved
    This update fixes a problem that caused issues with caching a SQL query that holds BLOB data types, therefore optimizing our database interactions.

  • Disallowing the export of empty messages in yii\log\FileTarget
    This PR prevents yii\log\FileTarget from exporting empty messages, eliminating unnecessary resource consumption and potential confusion.

  • Updates to yii.activeForm.js for better AJAX requests handling
    The code has been enhanced to handle aborting current AJAX requests when a new one is made, enhancing the responsiveness and overall user experience.

  • Introduction of currentAjaxRequest variable
    A new variable currentAjaxRequest has been introduced to keep track of the active AJAX request object, providing more granular control over ongoing AJAX requests.

  • Fixing the setting of aria-invalid attribute for inputs with errors
    The PR addresses issues with setting the aria-invalid attribute for input fields containing errors, thereby improving form interaction and overall accessibility.

@codecov
Copy link

codecov bot commented Jul 24, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (fe73d9c) 48.51% compared to head (cba3a5e) 48.96%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #19909      +/-   ##
==========================================
+ Coverage   48.51%   48.96%   +0.45%     
==========================================
  Files         445      445              
  Lines       42358    42840     +482     
==========================================
+ Hits        20548    20978     +430     
- Misses      21810    21862      +52     

see 15 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@samdark samdark requested review from a team July 24, 2023 14:03
@samdark
Copy link
Member

samdark commented Aug 18, 2023

@s1lver do you still remember this case? If so, would you please take a look?

@s1lver
Copy link
Member

s1lver commented Aug 18, 2023

@s1lver do you still remember this case? If so, would you please take a look?

hmm, 2017... yes, I looked at the related issue but I don't remember how I fixed it back then. I can just check again.

@s1lver
Copy link
Member

s1lver commented Aug 18, 2023

If confirm is used on the button and the option 'enableAjaxValidation' => true is enabled, then the form is not validated.

<?= Html::submitButton(
     'Submit',
     [
          'class' => 'btn btn-success',
          'data' => [
               'confirm' => 'Confirm message',
          ],
    ]
) ?>

I'm not sure if it can be accepted in this form. Would be nice to add tests

@samdark samdark added the pr:request for unit tests Unit tests are needed. label Aug 26, 2023
@samdark
Copy link
Member

samdark commented Aug 26, 2023

@tim-fischer-maschinensucher do you have time to add a test case to ensure it's not broken again ever?

@tim-fischer-maschinensucher
Copy link
Contributor Author

I should be able to get to it this or next week.

@tim-fischer-maschinensucher
Copy link
Contributor Author

I could not reproduce your issue @s1lver. I used the basic project and changed the contact form to fit your description, but everything seemed to work just fine.

@terabytesoftw
Copy link
Member

Hi, if we can't reproduce it, and everything seems to work fine, can we close it, and if the problem reappears do we open it again?

@s1lver
Copy link
Member

s1lver commented Sep 22, 2023

Hmm... I guess something else could have affected me. It's enough for me that there is a test for the old problem

@tim-fischer-maschinensucher
Copy link
Contributor Author

Is there anything I still need to do for this pr to progress?

@bizley
Copy link
Member

bizley commented Oct 17, 2023

Please resolve the conflict and I'll merge it.

@bizley bizley merged commit 4b423e4 into yiisoft:master Oct 17, 2023
50 checks passed
@bizley bizley added type:bug Bug and removed pr:request for unit tests Unit tests are needed. labels Oct 17, 2023
@bizley bizley added this to the 2.0.50 milestone Oct 17, 2023
@tim-fischer-maschinensucher tim-fischer-maschinensucher deleted the 13920-validation-marks-valid-field-as-invalid branch October 24, 2023 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants